To be "safe" and match what other mutating APIs do, AsSpan() needs to increment the version count.

AsReadOnlySpan does not have this requirement.

Is it really needed to increment the version stamp? This is unsafe API. You have to know what you are doing.

Is there a way for users to ensure the version count is incremented without us doing it here?

The assumption is that, even if you are using this API for perf benefits, you still want to do the "correct" thing with regards to other usages (such as fulfilling the IEnumerable<T> contract List<T> is currently exposing).

Its a trivial operation to increment the version count once and makes it more correct. For the case where you aren't mutating you can just get the ReadOnlySpan<T>. If you really want to silently mutate and avoid the version increment, you can then use MemoryMarshal to get a writeable span from the ROSpan.

Doesn't mimic List's behaviour; it should increment the version count on each change rather than just obtaining the Span window (which then may not change anything). That can't be done; also it would defeat the purpose of the Api.

That said if you are using the Span and enumerating the List at the same time; would be an unfortunate approach as enumerating over the Span will be much faster (as it can remove the bounds checks and skip the enumerator creation) and you already have that.

Doesn't mimic List's behaviour; it should increment the version count on each change rather than just obtaining the Span window (which then may not change anything).

Fair enough. The only downside is that it seems not ideal that users can't easily ensure "correct" versioning even if using the Span<T> APIs

Is it worth a comment that these fields are internal only for the use of CollectionsMarshal and to avoid further growing the generated code for List? It's not something we normally do.

nit: We should get in the habit of adding docs for parameters and return values (effectively all the xml fields that docs require). Something to consider going forward for new API additions (of course this applies to all future PRs and not just this one since we'd like to emphasize adding docs right in time and not accrue tech debt).

For example, see the following (which has param name and return value docs):
https://github.com/dotnet/dotnet-api-docs/blob/137fdd4f501ec90359833ad280064dbde43af7d3/xml/System.Collections/IList.xml#L213-L234
https://docs.microsoft.com/en-us/dotnet/api/system.collections.generic.icollection-1.contains?view=netframework-4.8

See the following for an example:
https://github.com/dotnet/corefx/blob/90f7cc01418c059f496528beeca77c5f5549e6fc/src/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.cs#L412-L432

cc @mairaw, @carlossanlop

Is there a way for us to detect and highlight the missing gaps in docs right in PRs to make it easier/discover-able for contributors?

We need null argument validation here, don't we?

When writing the tests; I realised I'd prefer this arrangement

public static Span<T> AsSpan<T>(List<T>? list)
    => new Span<T>(list?._items, 0, list?._size ?? 0);

Not sure how people would feel about that?

is that better than list is null ? Span<T>.Empty : new Span<T>(list._items, 0, list._size)?

Seems like the latter would have less null checks/etc

It's easier (at least for me) to reason about the code if written how @tannergooding suggested. I also would use default instead of Span<T>.Empty

public static Span<T> AsSpan<T>(List<T>? list)
    => list is null ? default : new Span<T>(list._items, 0, list._size);

I believe we prefer Span<T>.Empty over default based on the existing usages

It's easier to see call sites of Span<T>.Empty than usage of default, but either works.

We have default being used in similar domain here:

There are only a few hits of Span<T>.Empty:
https://source.dot.net/#System.Private.CoreLib/shared/System/Span.cs,a02fdd2de3235cda,references

Although, a lot more for ReadOnlySpan<T>.Empty:
https://source.dot.net/#System.Private.CoreLib/shared/System/ReadOnlySpan.cs,47744f8b41c9226f,references